Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support modulo operator #27

Merged
merged 1 commit into from
Nov 5, 2020
Merged

Support modulo operator #27

merged 1 commit into from
Nov 5, 2020

Conversation

eecheng87
Copy link
Collaborator

No description provided.

@eecheng87 eecheng87 force-pushed the mod branch 2 times, most recently from b2a486b to cf86617 Compare November 5, 2020 09:07
src/codegen.c Outdated Show resolved Hide resolved
@@ -367,6 +368,14 @@ void code_generate()
if (dump_ir == 1)
printf(" x%d /= x%d", dest_reg, OP_reg);
break;
case OP_mod:
emit(__div(__AL, OP_reg + 1, OP_reg, dest_reg));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both div and mul instruction are heavier. Instead, you can add shortcut for power-of-2. That is, convert the original modulo operation into n & (d - 1) if d is power-of-2. You should implement the shortcut in src/cfront.c.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think only global expression can do this shortcut because shecc can't determine the value of operand in local expression before execution time. Can we ignore optimization for local expression in this patch?

BTW, relative issue#28 and detailed information was sent.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let's merge this change first. Do append the comments with TODO: optimize ...

@jserv jserv merged commit d908cfe into sysprog21:master Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants